Skip to content

fix(mcp): Fix update_task signature and MCP instructions#421

Merged
Wirasm merged 3 commits intomainfrom
fix/mcp-instructions-and-update-task
Aug 21, 2025
Merged

fix(mcp): Fix update_task signature and MCP instructions#421
Wirasm merged 3 commits intomainfrom
fix/mcp-instructions-and-update-task

Conversation

@Wirasm
Copy link
Copy Markdown
Collaborator

@Wirasm Wirasm commented Aug 21, 2025

Pull Request

Summary

Fixes issue #420 where updating tasks creates new tasks instead of modifying existing ones. The root cause was incorrect MCP instructions and inconsistent function signatures.

Changes Made

  • Fixed update_task function signature to use individual optional parameters instead of TypedDict
  • Updated MCP instructions to use correct function names (replaced non-existent manage_task with actual functions)
  • Added complete function signatures for all MCP tools in the instructions
  • Improved workflow documentation with concrete examples

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Performance improvement
  • Code refactoring

Affected Services

  • Frontend (React UI)
  • Server (FastAPI backend)
  • MCP Server (Model Context Protocol)
  • Agents (PydanticAI service)
  • Database (migrations/schema)
  • Docker/Infrastructure
  • Documentation site

Testing

  • All existing tests pass
  • Added new tests for new functionality
  • Manually tested affected user flows
  • Docker builds succeed for all services

Test Evidence

# Syntax validation passed
python3 -m py_compile python/src/mcp_server/features/tasks/task_tools.py
python3 -m py_compile python/src/mcp_server/mcp_server.py

# Manual testing of update_task function
# Successfully updated task with new signature
mcp__archon__update_task(task_id="...", title="...", status="doing")

Checklist

  • My code follows the service architecture patterns
  • If using an AI coding assistant, I used the CLAUDE.md rules
  • I have added tests that prove my fix/feature works
  • All new and existing tests pass locally
  • My changes generate no new warnings
  • I have updated relevant documentation
  • I have verified no regressions in existing features

Breaking Changes

None - The changes maintain backward compatibility by using optional parameters.

Additional Notes

This PR resolves two related issues:

  1. Wrong function names in MCP instructions: The instructions referenced manage_task and manage_project functions that don't exist
  2. Inconsistent update function signatures: update_task used TypedDict while update_project and update_document used individual parameters

The fix ensures all update functions follow the same pattern and AI agents have clear, accurate instructions for using the MCP tools.

Fixes #420

Summary by CodeRabbit

  • Refactor

    • Task update now uses explicit optional fields (e.g., title, description, status, assignee, order, feature, sources, code examples) instead of a single dictionary; only provided fields are sent.
    • Calling without fields returns a validation error.
    • Breaking change for clients using the old dictionary-style updates.
  • Documentation

    • Rewrote MCP instructions into a clearer, API-oriented guide covering task/project/document/version management, research patterns, and workflow rules.
  • Tests

    • Updated tests for the new task update API and added coverage for empty-update validation.

Resolves #420 - Tasks being duplicated instead of updated

Changes:
1. Fixed update_task function signature to use individual optional parameters
   - Changed from TypedDict to explicit parameters (title, status, etc.)
   - Consistent with update_project and update_document patterns
   - Builds update_fields dict internally from provided parameters

2. Updated MCP instructions with correct function names
   - Replaced non-existent manage_task with actual functions
   - Added complete function signatures for all tools
   - Improved workflow documentation with concrete examples

This fixes the issue where AI agents were confused by:
- Wrong function names in instructions (manage_task vs update_task)
- Inconsistent parameter patterns across update functions
- TypedDict magic that wasn't clearly documented

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Aug 21, 2025

Walkthrough

Refactors update_task to accept explicit optional fields and builds the update payload internally with validation for empty updates. Removes the TypedDict alias. Updates tests to the new signature and validates request JSON. Modifies MCP_INSTRUCTIONS text only; no runtime logic changes elsewhere.

Changes

Cohort / File(s) Summary
Task tools API refactor
python/src/mcp_server/features/tasks/task_tools.py
Replaced TypedDict-based update_fields with explicit optional params; constructs payload internally; adds no-fields validation; removes TypedDict import/type; updates docstrings/examples.
MCP instructions text
python/src/mcp_server/mcp_server.py
Rewrites MCP_INSTRUCTIONS content and structure; no logic or control-flow changes.
Tests adjusted for new API
python/tests/mcp_server/features/tasks/test_task_tools.py
Updates calls to new update_task signature; asserts PUT JSON body contents; adds test for no-fields validation_error.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client
  participant Tools as task_tools.update_task
  participant API as HTTP API /api/tasks/{task_id}

  Client->>Tools: update_task(task_id, [title|description|status|...])
  Tools->>Tools: Build update_fields from provided kwargs
  alt No fields provided
    Tools-->>Client: validation_error ("No fields to update")
  else At least one field
    Tools->>API: PUT /api/tasks/{task_id} with JSON(update_fields)
    API-->>Tools: 200/4xx response
    Tools-->>Client: Result or error
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Assessment against linked issues

Objective Addressed Explanation
Ensure updating a task changes its status instead of creating a new task (#420) Client now sends only provided fields and blocks empty updates; no evidence of server-side duplication fix.
Prevent updates with empty payloads leading to unintended behavior (#420)
Maintain existing PUT /api/tasks/{task_id} update semantics (#420)

Poem

I thump my paw: “Update, not create!”
Fields hop in, none left to fate.
No empty baskets, payloads neat,
PUT down the path with tidy feet.
Tests nibble greens, all checks align—
Tasks turn to “doing,” then “done” just fine. 🥕🐇

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/mcp-instructions-and-update-task

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Wirasm added 2 commits August 21, 2025 20:26
- Fixed test_update_task_status to use individual parameters
- Added test_update_task_no_fields for validation testing
- All MCP tests passing (44 tests)
- Remove trailing whitespace
- Consistent formatting in instruction text
@Wirasm Wirasm marked this pull request as ready for review August 21, 2025 17:40
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (6)
python/src/mcp_server/features/tasks/task_tools.py (4)

10-10: Import Literal for stronger type-safety on status.

Prepare for a Literal-typed status parameter in update_task.

-from typing import Any, Dict, List, Optional
+from typing import Any, Dict, List, Optional, Literal

305-313: Signature change LGTM; consider constraining status to allowed literals.

Using Literal reduces typos and improves mypy checks while remaining backward-compatible.

-        status: Optional[str] = None,
+        status: Optional[Literal["todo", "doing", "review", "done"]] = None,

339-357: Type the payload and simplify construction of update_fields.

Explicit typing helps mypy; the dict-comprehension reduces repetition and mistakes when new fields are added.

-            # Build update_fields dict from provided parameters
-            update_fields = {}
-            if title is not None:
-                update_fields["title"] = title
-            if description is not None:
-                update_fields["description"] = description
-            if status is not None:
-                update_fields["status"] = status
-            if assignee is not None:
-                update_fields["assignee"] = assignee
-            if task_order is not None:
-                update_fields["task_order"] = task_order
-            if feature is not None:
-                update_fields["feature"] = feature
-            if sources is not None:
-                update_fields["sources"] = sources
-            if code_examples is not None:
-                update_fields["code_examples"] = code_examples
+            # Build update_fields dict from provided parameters
+            update_fields: Dict[str, Any] = {
+                k: v
+                for k, v in {
+                    "title": title,
+                    "description": description,
+                    "status": status,
+                    "assignee": assignee,
+                    "task_order": task_order,
+                    "feature": feature,
+                    "sources": sources,
+                    "code_examples": code_examples,
+                }.items()
+                if v is not None
+            }

365-379: Consider accepting 204 No Content for robustness.

Some APIs return 204 for successful updates. Handling 200 and 204 avoids false negatives and reduces coupling to response shape.

-                response = await client.put(
-                    urljoin(api_url, f"/api/tasks/{task_id}"), json=update_fields
-                )
-
-                if response.status_code == 200:
-                    result = response.json()
-                    return json.dumps({
-                        "success": True,
-                        "task": result.get("task"),
-                        "message": result.get("message", "Task updated successfully"),
-                    })
+                response = await client.put(
+                    urljoin(api_url, f"/api/tasks/{task_id}"), json=update_fields
+                )
+
+                if response.status_code in (200, 204):
+                    if response.status_code == 204 or not response.content:
+                        return json.dumps({
+                            "success": True,
+                            "task": None,
+                            "message": "Task updated successfully",
+                        })
+                    result = response.json()
+                    return json.dumps({
+                        "success": True,
+                        "task": result.get("task"),
+                        "message": result.get("message", "Task updated successfully"),
+                    })
python/tests/mcp_server/features/tasks/test_task_tools.py (1)

185-204: Great coverage for no-fields guard; optionally assert no HTTP call is made.

You can also assert that PUT is never invoked in this path to harden the contract.

 @pytest.mark.asyncio
 async def test_update_task_no_fields(mock_mcp, mock_context):
     """Test updating task with no fields returns validation error."""
     register_task_tools(mock_mcp)
 
     # Get the update_task function
     update_task = mock_mcp._tools.get("update_task")
 
     assert update_task is not None, "update_task tool not registered"
 
-    # Call update_task with no optional fields
-    result = await update_task(mock_context, task_id="task-123")
+    # Call update_task with no optional fields
+    with patch("src.mcp_server.features.tasks.task_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+        result = await update_task(mock_context, task_id="task-123")
 
     result_data = json.loads(result)
     assert result_data["success"] is False
     assert "error" in result_data
     assert isinstance(result_data["error"], dict)
     assert result_data["error"]["type"] == "validation_error"
     assert "No fields to update" in result_data["error"]["message"]
+
+    # Ensure we never attempted an HTTP call
+    mock_async_client.put.assert_not_called()
python/src/mcp_server/mcp_server.py (1)

216-221: Inline API snippet for update_task: list all supported fields for parity.

Minor nit: documenting all optional params avoids confusion and aligns with task_tools.update_task.

 - `update_task(task_id, title=None, status=None, assignee=None, ...)`
+ - `update_task(task_id, title=None, description=None, status=None, assignee=None, task_order=None, feature=None, sources=None, code_examples=None)`
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6f7c08e and 2532a09.

📒 Files selected for processing (3)
  • python/src/mcp_server/features/tasks/task_tools.py (2 hunks)
  • python/src/mcp_server/mcp_server.py (1 hunks)
  • python/tests/mcp_server/features/tasks/test_task_tools.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
python/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

python/**/*.py: Target Python 3.12 with a 120-character line length
Use Ruff for linting and Mypy for type checking before commit

Files:

  • python/src/mcp_server/mcp_server.py
  • python/src/mcp_server/features/tasks/task_tools.py
  • python/tests/mcp_server/features/tasks/test_task_tools.py
{python/**/*.py,archon-ui-main/src/**/*.{ts,tsx,js,jsx}}

📄 CodeRabbit inference engine (CLAUDE.md)

{python/**/*.py,archon-ui-main/src/**/*.{ts,tsx,js,jsx}}: Remove dead code immediately; do not keep legacy/unused functions
Avoid comments that reference change history (e.g., LEGACY, CHANGED, REMOVED); keep comments focused on current functionality

Files:

  • python/src/mcp_server/mcp_server.py
  • python/src/mcp_server/features/tasks/task_tools.py
  • python/tests/mcp_server/features/tasks/test_task_tools.py
python/tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Place backend tests under python/tests/

Files:

  • python/tests/mcp_server/features/tasks/test_task_tools.py
🧠 Learnings (1)
📚 Learning: 2025-08-21T11:22:33.541Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-21T11:22:33.541Z
Learning: Applies to python/src/mcp/** : Keep MCP server implementation under python/src/mcp/

Applied to files:

  • python/src/mcp_server/mcp_server.py
🧬 Code graph analysis (2)
python/src/mcp_server/features/tasks/task_tools.py (2)
python/src/mcp_server/utils/timeout_config.py (1)
  • get_default_timeout (13-31)
python/src/mcp_server/utils/error_handling.py (2)
  • MCPErrorFormatter (16-148)
  • format_error (20-57)
python/tests/mcp_server/features/tasks/test_task_tools.py (2)
python/tests/mcp_server/features/documents/test_document_tools.py (2)
  • mock_context (31-33)
  • mock_mcp (13-27)
python/src/mcp_server/features/tasks/task_tools.py (1)
  • update_task (302-386)
🔇 Additional comments (7)
python/src/mcp_server/features/tasks/task_tools.py (1)

358-364: Good: early validation prevents accidental no-op updates.

This avoids confusing “successful” calls that don’t change anything and provides clear guidance to callers.

python/tests/mcp_server/features/tasks/test_task_tools.py (1)

171-172: LGTM: verifies payload fields for update_task.

Asserting the exact JSON sent (status, assignee) catches regressions in request construction.

Also applies to: 178-183

python/src/mcp_server/mcp_server.py (5)

195-199: Docs: Clear rule to use MCP tools for task management.

This addresses the root cause from #420 (wrong tool names) by steering clients to the correct surface.


206-214: Task Management Cycle reads well and matches exported tools.

The examples use get_task and update_task consistently with the implemented tool names.


245-249: Status flow guidance is consistent and actionable.

Matches the allowed status values used in task_tools.py.


239-243: Research tool availability verified

The functions perform_rag_query, search_code_examples, and get_available_sources are all implemented and decorated with @mcp.tool() in python/src/mcp_server/modules/rag_module.py. No further action is needed.


250-256: All Version Management Tools Properly Registered

I’ve confirmed that the four tools—create_version, list_versions, get_version, and restore_version—are defined in version_tools.py with @mcp.tool() and that register_version_tools is imported and invoked in mcp_server.py, with corresponding tests exercising each registration. No further changes are needed here.

Copy link
Copy Markdown
Collaborator

@sean-esk sean-esk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Wirasm this looks good to me, this was the exact fix I had to do for the typedict, so I say ship it since it's broken on main.

@Wirasm Wirasm merged commit 26a9332 into main Aug 21, 2025
8 checks passed
@Wirasm Wirasm deleted the fix/mcp-instructions-and-update-task branch August 21, 2025 19:11
@Wirasm
Copy link
Copy Markdown
Collaborator Author

Wirasm commented Aug 22, 2025

resolves: #405

coleam00 pushed a commit that referenced this pull request Apr 7, 2026
…e cleanup (#601)

* feat(cli): add archon complete <branch> command for worktree lifecycle cleanup (#421)

After merging a PR, users had to run three separate commands to clean up
the worktree, local branch, and remote branch. This adds a single
top-level command that handles the full lifecycle in one step.

Changes:
- Add findActiveByBranchName() to isolation-environments.ts for DB lookup by branch name
- Export removeEnvironment and RemoveEnvironmentOptions from @archon/core index
- Add isolationCompleteCommand() to packages/cli/src/commands/isolation.ts with
  explicit uncommitted-changes check and --force flag support
- Add 'complete' case to cli.ts with routing, help text, and import
- Add isolation.test.ts with 6 tests covering all edge cases
- Split CLI test batches in package.json to avoid mock.module() pollution

Fixes #421

* fix(cli): address review findings for archon complete command

- Remove unused removeEnvironment/RemoveEnvironmentOptions re-exports from
  @archon/core index (YAGNI — caller already uses deep path import)
- Add ORDER BY e.created_at DESC to findActiveByBranchName for deterministic
  results when multiple active environments share a branch name
- Wrap findActiveByBranchName in per-branch try/catch so DB errors are
  attributed to the specific branch and the summary still prints
- Update inline comment to describe all silent-return conditions in
  removeEnvironment, not just the uncommitted-changes path
- Update findActiveByBranchName docstring to document the JOIN and extra
  codebase_default_cwd field in the return type
- Update file-level docstring in isolation.ts to include complete command
- Document archon complete command in CLAUDE.md, cli-user-guide.md,
  getting-started.md, and cli-developer-guide.md
- Remove unnecessary String() coercions in summary output line
Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026
…e cleanup (coleam00#601)

* feat(cli): add archon complete <branch> command for worktree lifecycle cleanup (coleam00#421)

After merging a PR, users had to run three separate commands to clean up
the worktree, local branch, and remote branch. This adds a single
top-level command that handles the full lifecycle in one step.

Changes:
- Add findActiveByBranchName() to isolation-environments.ts for DB lookup by branch name
- Export removeEnvironment and RemoveEnvironmentOptions from @archon/core index
- Add isolationCompleteCommand() to packages/cli/src/commands/isolation.ts with
  explicit uncommitted-changes check and --force flag support
- Add 'complete' case to cli.ts with routing, help text, and import
- Add isolation.test.ts with 6 tests covering all edge cases
- Split CLI test batches in package.json to avoid mock.module() pollution

Fixes coleam00#421

* fix(cli): address review findings for archon complete command

- Remove unused removeEnvironment/RemoveEnvironmentOptions re-exports from
  @archon/core index (YAGNI — caller already uses deep path import)
- Add ORDER BY e.created_at DESC to findActiveByBranchName for deterministic
  results when multiple active environments share a branch name
- Wrap findActiveByBranchName in per-branch try/catch so DB errors are
  attributed to the specific branch and the summary still prints
- Update inline comment to describe all silent-return conditions in
  removeEnvironment, not just the uncommitted-changes path
- Update findActiveByBranchName docstring to document the JOIN and extra
  codebase_default_cwd field in the return type
- Update file-level docstring in isolation.ts to include complete command
- Document archon complete command in CLAUDE.md, cli-user-guide.md,
  getting-started.md, and cli-developer-guide.md
- Remove unnecessary String() coercions in summary output line
joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026
…e cleanup (coleam00#601)

* feat(cli): add archon complete <branch> command for worktree lifecycle cleanup (coleam00#421)

After merging a PR, users had to run three separate commands to clean up
the worktree, local branch, and remote branch. This adds a single
top-level command that handles the full lifecycle in one step.

Changes:
- Add findActiveByBranchName() to isolation-environments.ts for DB lookup by branch name
- Export removeEnvironment and RemoveEnvironmentOptions from @archon/core index
- Add isolationCompleteCommand() to packages/cli/src/commands/isolation.ts with
  explicit uncommitted-changes check and --force flag support
- Add 'complete' case to cli.ts with routing, help text, and import
- Add isolation.test.ts with 6 tests covering all edge cases
- Split CLI test batches in package.json to avoid mock.module() pollution

Fixes coleam00#421

* fix(cli): address review findings for archon complete command

- Remove unused removeEnvironment/RemoveEnvironmentOptions re-exports from
  @archon/core index (YAGNI — caller already uses deep path import)
- Add ORDER BY e.created_at DESC to findActiveByBranchName for deterministic
  results when multiple active environments share a branch name
- Wrap findActiveByBranchName in per-branch try/catch so DB errors are
  attributed to the specific branch and the summary still prints
- Update inline comment to describe all silent-return conditions in
  removeEnvironment, not just the uncommitted-changes path
- Update findActiveByBranchName docstring to document the JOIN and extra
  codebase_default_cwd field in the return type
- Update file-level docstring in isolation.ts to include complete command
- Document archon complete command in CLAUDE.md, cli-user-guide.md,
  getting-started.md, and cli-developer-guide.md
- Remove unnecessary String() coercions in summary output line
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 [Bug]: Updating a task creates a new task with backlog status instead of changing the status of the task worked on

2 participants